Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Sep 1, 2018

With this commit, we get a new prompt like:

SSH Public Key:
Enter an empty string, your public key (e.g. 'ssh-rsa AAAA...'), or one of the following numbers:
1: /home/trking/.ssh/id_rsa.pub

This commit also adds an "is this SSH pubkey valid?" check based on our previous validation.

CC @yifan-gu.

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 1, 2018
@wking wking force-pushed the ssh-prompts branch 2 times, most recently from fd574cf to b7bce87 Compare September 2, 2018 04:26
The test suite imports the TLS asset package, but keeping that import
out of the validate package itself allows us to import the validate
package from pkg/asset and descendants without having to worry about
cyclic imports.

I've moved the lastIP test into a separate file (which stays in the
validate package) so I can test the helper without making it part of
the public API.

The BUILD.bazel change was generated with:

  $ bazel run //:gazelle
With this commit, we get a new prompt like:

  SSH Public Key:
  Enter an empty string, your public key (e.g. 'ssh-rsa AAAA...'), or one of the following numbers:
  1: /home/trking/.ssh/id_rsa.pub

This commit also adds an "is this SSH pubkey valid?" check based on
our previous validation.

The BUILD.bazel change was generated with:

  $ bazel run //:gazelle

The testify addition there is catching up with 345d843 (asset: Only
persistent data to disk for targetable assets, 2018-08-30, openshift#196).
@yifan-gu
Copy link
Contributor

yifan-gu commented Sep 4, 2018

A unit test to test the input reading would be nice :)

Copy link
Contributor

@yifan-gu yifan-gu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 4, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wking, yifan-gu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wking
Copy link
Member Author

wking commented Sep 4, 2018

I'm not very clear on what's going on with these CI failure. The ShellCheck and YAML lint jobs are reporting an error in the GitHub UI here, but have no finished.json here or here. The Terraform format job complains about:

Unable to load build details from gs://origin-ci-test/pr-logs/pull/openshift_installer/199/pull-ci-origin-installer-tf-fmt/335

but that job has no artifacts outside of logs/ (it usually looks like this). If nobody chimes in with more directed ideas in the next hour or so, I'll just kick off a retest and hope ;).

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 136752a into openshift:master Sep 4, 2018
@wking wking deleted the ssh-prompts branch September 4, 2018 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants